Skip to content

Respect pre-set env vars and add cgroup memory detection in env scripts#17765

Open
MileaRobertStefan wants to merge 3 commits into
apache:masterfrom
MileaRobertStefan:fix_env_scripts_memory_size_envvar
Open

Respect pre-set env vars and add cgroup memory detection in env scripts#17765
MileaRobertStefan wants to merge 3 commits into
apache:masterfrom
MileaRobertStefan:fix_env_scripts_memory_size_envvar

Conversation

@MileaRobertStefan
Copy link
Copy Markdown

@MileaRobertStefan MileaRobertStefan commented May 25, 2026

Description

Adds cgroup v1/v2 memory detection.

This PR has:

  • been self-reviewed.
    • concurrent read
    • concurrent write
    • concurrent read and write
  • added documentation for new or modified features or behaviors.
  • added Javadocs for most classes and all non-trivial methods.
  • added or updated version, license, or notice information
  • added comments explaining the "why" and the intent of the code wherever would not be obvious
    for an unfamiliar reader.
  • added unit tests or modified existing tests to cover new code paths, ensuring the threshold
    for code coverage.
  • added integration tests.
  • been tested in a test IoTDB cluster.

Copy link
Copy Markdown
Contributor

@CRZbulabula CRZbulabula left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for tackling this — JVM auto-sizing in containers reading host memory is a real and well-known footgun, and the cgroup v1/v2 dual handling here is the right shape. A few inline suggestions below; nothing blocking. Two file-level notes:

  1. No automated tests. Cgroup-gated shell behavior is awkward to unit-test, but a small harness that parameterizes the cgroup mount point (default /sys/fs/cgroup) and runs the function against fixture files would catch regressions cheaply. Acceptable to skip given the PR description confirms manual cluster verification.
  2. Duplication. The same ~17-line block is copy-pasted across confignode-env.sh and datanode-env.sh. The existing calculate_memory_sizes() is already duplicated in both files, so this is consistent with the current style — but a follow-up that sources a shared helper from scripts/conf/iotdb-common.sh would reduce drift.
  3. AINode unaffected, right? scripts/sbin/start-ainode.sh is Python-based and presumably doesn't share this Bash sizing machinery — worth confirming so nobody assumes container-aware sizing applies there too.

Comment thread scripts/conf/confignode-env.sh Outdated
Comment thread scripts/conf/confignode-env.sh Outdated
Comment thread scripts/conf/datanode-env.sh Outdated
Comment thread scripts/conf/datanode-env.sh Outdated
Comment thread scripts/conf/windows/confignode-env.bat Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants